C#: Models for higher order methods. #17742
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
michaelnebel
force-pushed
the
csharp/higherordermodels
branch
from
f25e44b to
21a0f76
Compare
michaelnebel
force-pushed
the
csharp/higherordermodels
branch
4 times, most recently
from
cb0b639 to
eb1c4e9
Compare
michaelnebel
added
the
no-change-note-required
| TCapturedVariableContent(VariableCapture::CapturedVariable v) | ||
| TCapturedVariableContent(VariableCapture::CapturedVariable v) or | ||
| TDelegateCallArgumentContent(int i) { | ||
| i = [0 .. max(int j | j = any(DelegateCall dc).getNumberOfArguments())] |
There was a problem hiding this comment.
nit: max(any(DelegateCall dc).getNumberOfArguments()) - 1.
| * If there is a delegate call f(x), then we store "x" on "f" | ||
| * using a delegate argument content set. | ||
| */ | ||
| private predicate storeStepDelegateCall(Node node1, ContentSet c, Node node2) { |
There was a problem hiding this comment.
This implementation does not properly take CFG splitting into account. I think the following works:
private predicate storeStepDelegateCall(ExplicitArgumentNode node1, ContentSet c, Node node2) {
exists(ExplicitDelegateLikeDataFlowCall call, int i |
node1.argumentOf(call, TPositionalArgumentPosition(i)) and
lambdaCall(call, _, node2.(PostUpdateNode).getPreUpdateNode()) and
c.isDelegateCallArgument(i)
)
}There was a problem hiding this comment.
Uh, thx!
Could you elaborate a bit on, which cases are now covered that wasn't covered before?
There was a problem hiding this comment.
The old implementation had the issue that it could potentially mix CFG nodes belonging to different CFG splits. Moreover, using DelegateLikeCall instead of just DelegateCall means that function pointers are also taken into account.
| * If there is a delegate call f(x), then we read the return of the delegate | ||
| * call. | ||
| */ | ||
| private predicate readStepDelegateCall(Node node1, ContentSet c, Node node2) { |
There was a problem hiding this comment.
Again, because of CFG splitting:
private predicate readStepDelegateCall(Node node1, ContentSet c, OutNode node2) {
exists(ExplicitDelegateLikeDataFlowCall call |
lambdaCall(call, _, node1) and
node2.getCall(TNormalReturnKind()) = call and
c.isDelegateCallReturn()
)
}| // neutral=Models;HigherOrderParameters;Map;(System.Func<System.Object,System.Object>,System.Object);summary;df-generated | ||
| // contentbased-summary=Models;HigherOrderParameters;false;Map;(System.Func<System.Object,System.Object>,System.Object);;Argument[1];Argument[0].Parameter[0];value;dfc-generated | ||
| // contentbased-summary=Models;HigherOrderParameters;false;Map;(System.Func<System.Object,System.Object>,System.Object);;Argument[0].ReturnValue;ReturnValue;value;dfc-generated | ||
| public object Map(Func<object, object> f, object o) |
There was a problem hiding this comment.
Nit: Apply would be a more appropriate name.
| // neutral=Models;HigherOrderParameters;Map2;(System.Object,System.Func<System.Object,System.Object,System.Object>);summary;df-generated | ||
| // contentbased-summary=Models;HigherOrderParameters;false;Map2;(System.Object,System.Func<System.Object,System.Object,System.Object>);;Argument[0];Argument[1].Parameter[1];value;dfc-generated | ||
| // contentbased-summary=Models;HigherOrderParameters;false;Map2;(System.Object,System.Func<System.Object,System.Object,System.Object>);;Argument[1].ReturnValue;ReturnValue;value;dfc-generated | ||
| public object Map2(object o, Func<object, object, object> f) |
| } | ||
|
|
||
| /** | ||
| * Models as Data currently doesn't support callback logic in fields. |
There was a problem hiding this comment.
I don't think that's true: For example, I would expect something like this to work just fine:
input = Argument[0].Field[Foo].ReturnValue
output = ReturnValue
There was a problem hiding this comment.
I tried that but it didn't work. Furthermore, if Foo is a synthetic field some of the data flow consistency tests start failing. Due to time constraints (and since wasn't that many summaries), I just decided to remove the problematic summaries from the output.
In any case, I will change the wording.
…synthetic fields.
michaelnebel
force-pushed
the
csharp/higherordermodels
branch
from
eb1c4e9 to
e9c9519
Compare
| @@ -1149,7 +1149,7 @@ private module Cached { | |||
| } or | |||
| TCapturedVariableContent(VariableCapture::CapturedVariable v) or | |||
| TDelegateCallArgumentContent(int i) { | |||
| i = [0 .. max(int j | j = any(DelegateCall dc).getNumberOfArguments())] | |||
| i = [0 .. max(any(DelegateCall dc).getNumberOfArguments())] | |||
There was a problem hiding this comment.
Ah yes, you are right! Thank you for paying close attention!
There was a problem hiding this comment.
I suppose we also want to consider DelegateLikeCall instead.
In this PR we added support for read/store steps for delegate calls (where the delegate being called is parameter of a callable) for C#.
The read and store steps for delegate calls and delegate returns are basically "dead ends" and are only relevant for content dataflow related to model generation.